Skip to content

Conversation

@susnux
Copy link
Contributor

@susnux susnux commented Nov 6, 2025

@susnux susnux requested a review from ShGKme November 6, 2025 00:57
@susnux susnux added bug Something isn't working 3. to review labels Nov 6, 2025
@Antreesy
Copy link
Contributor

Antreesy commented Nov 6, 2025

IDK, are there options to keep/enforce multiline there, e.g. by line length?
image

@susnux
Copy link
Contributor Author

susnux commented Nov 6, 2025

options to keep/enforce multiline there

The old value was not multi line (considered by the fixer) because brackets and first / last item are on the same line.
What is available is "maxItems" for single line.

@Antreesy
Copy link
Contributor

Antreesy commented Nov 6, 2025

This? https://eslint.style/rules/list-style#maxitems

Looks nice, I'd consider great if limited to 3 items, maybe?

@susnux
Copy link
Contributor Author

susnux commented Nov 6, 2025

Looks nice, I'd consider great if limited to 3 items, maybe?

Pushed a commit

Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Now it also affects functions with 4+ arguments... But at least consistent =)
LGTM, I would wait for the second opinion to merge

@susnux
Copy link
Contributor Author

susnux commented Nov 6, 2025

Now it also affects functions with 4+ arguments... But at least consistent =)

Maybe exclude functions

@ShGKme
Copy link
Contributor

ShGKme commented Nov 6, 2025

The old value was not multi line (considered by the fixer) because brackets and first / last item are on the same line.
What is available is "maxItems" for single line.

I'm voting for the previous behavior.

If a developer makes it multiline — keep multiline. If single-line - keep single line.

The decision here doesn't depend on the number of items but on the nature of the array.

@ShGKme
Copy link
Contributor

ShGKme commented Nov 6, 2025

In the referenced PR the tests were adjusted to be more real life like, with som explanations

@susnux
Copy link
Contributor Author

susnux commented Nov 6, 2025

@ShGKme I added the tests you created to this PR see the two commits to see how it is changed

Comment on lines 10 to 16
const LATIN_VOWELS = [
'a',
'e',
'i',
'o',
'u',
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need diff-safe here? I don't think new vowels are expected to be added 👀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants